Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(elements): avoid exception when window is undefined #23324

Closed
wants to merge 1 commit into from
Closed

fix(elements): avoid exception when window is undefined #23324

wants to merge 1 commit into from

Conversation

vikerman
Copy link
Contributor

Detect server environment by checking typeof window and schedule render immediately instead of waiting for RAF.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?
Avoid window is undefined exception when running Angular Elements on platform-server.

This doesn't make the Angular elements work on the server yet but just a first step.

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

@mary-poppins
Copy link

You can preview 0b3d636 at https://pr23324-0b3d636.ngbuilds.io/.

@vikerman vikerman changed the title fix(elements): fix for SSR fix(elements): avoid exception when window is undefined Apr 11, 2018
@vikerman vikerman added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 11, 2018
@mary-poppins
Copy link

You can preview 62cfe82 at https://pr23324-62cfe82.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 659b0b8 at https://pr23324-659b0b8.ngbuilds.io/.

Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no test? how will we ensure that this remains working over time? can you please add a test?

@vikerman
Copy link
Contributor Author

vikerman commented Apr 11, 2018

For tests - We need to run the Custom Elements test suite as Node tests. Which involves making the custom elements work on platform-server first. I will do that post-6.0.

For now this change enables us to polyfill Domino at the app level to get custom elements working on the server.

@vikerman vikerman added target: major This PR is targeted for the next major release and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 11, 2018
Detect server environment by checking `typeof window` and schedule render immediately instead of waiting for RAF.

This does not make Angular Elements work on platform-server. This is just the first step.
@vikerman vikerman removed the target: major This PR is targeted for the next major release label Apr 12, 2018
@mary-poppins
Copy link

You can preview 2b5cb64 at https://pr23324-2b5cb64.ngbuilds.io/.

@vikerman
Copy link
Contributor Author

vikerman commented Apr 12, 2018

Fix verified with SSR example (at http://go/ce-ssr)

@vikerman vikerman added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Apr 12, 2018
@IgorMinar IgorMinar closed this in af46d09 Apr 12, 2018
@vikerman vikerman deleted the ce-server branch April 12, 2018 15:33
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018
Detect server environment by checking `typeof window` and schedule render immediately instead of waiting for RAF.

This does not make Angular Elements work on platform-server. This is just the first step.

PR Close angular#23324
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018
Detect server environment by checking `typeof window` and schedule render immediately instead of waiting for RAF.

This does not make Angular Elements work on platform-server. This is just the first step.

PR Close angular#23324
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018
Detect server environment by checking `typeof window` and schedule render immediately instead of waiting for RAF.

This does not make Angular Elements work on platform-server. This is just the first step.

PR Close angular#23324
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants